Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge FEGA related components from pipeline into SDA #249

Closed
wants to merge 34 commits into from

Conversation

jbygdell
Copy link
Collaborator

@jbygdell jbygdell commented Aug 9, 2023

This merges the components needed for a federated setup from sda-pipeline into sda i.e. finalize, ingest, intercept, mapper, verify.

  • Backup is merged with finalize in order to simplify the message flow.
  • Static code tests uses dockerized services instead of mocked backends for DB, MQ and S3.
  • Functionality tests track the entire path from upload to complete ingestion and dataset release/deprecation.
  • Removes code and functionality tests for the sda-pipeline folder.
  • Updates the helm charts

related to #14

@jbygdell jbygdell self-assigned this Aug 9, 2023
@jbygdell jbygdell force-pushed the merge_pipeline branch 5 times, most recently from 09e72c3 to e44fee8 Compare August 10, 2023 08:04
@jbygdell jbygdell force-pushed the merge_pipeline branch 12 times, most recently from 3cb0959 to ca7f269 Compare August 23, 2023 06:35
@jbygdell jbygdell changed the title Merge pipeline Merge pipeline into SDA Aug 23, 2023
@jbygdell jbygdell requested a review from a team August 23, 2023 08:45
@jbygdell jbygdell marked this pull request as ready for review August 23, 2023 08:45
@pahatz
Copy link
Contributor

pahatz commented Aug 24, 2023

Could you elaborate a bit on how these reflect on the commits of the PR?

Static code tests uses dockerized services instead of mocked backends for DB, MQ and S3.
Functionality tests track the entire path from upload to complete ingestion and dataset release/deprecation.

also why is this needed?

Removes code and functionality tests for the sda-pipeline folder.

This could maybe benefit from being an epic or at least multiple PRs but it's a bit late now :)

I was also expecting a whole different thing from the title of the ticket. Are we archiving the sda-pipeline repo after this? Would that mean that we're not keeping any history of the merged repo?

@jbygdell jbygdell changed the title Merge pipeline into SDA Merge pipeline into SDA, part 1 Aug 24, 2023
Copy link
Contributor

@blankdots blankdots left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Backup is merged with finalize in order to simplify the message flow.

so the finalize service also performs the backup if a backup storage is configured, do i understand this correctly ?
seems to also include the sftp backup type which i assume is for mirroring

When deploying shall we set up one or more finalize services, each doing different functionalities ? i assume one (maybe 2 for mirroring use case ?), but want to be sure

shall we include this in the description and maybe adjust finalize.md ?

@jbygdell
Copy link
Collaborator Author

Backup is merged with finalize in order to simplify the message flow.

so the finalize service also performs the backup if a backup storage is configured, do i understand this correctly ? seems to also include the sftp backup type which i assume is for mirroring

Precisely, the merge is primarily because there is no dedicated backup message.
sftp is included since it exists as a storage backed.

When deploying shall we set up one or more finalize services, each doing different functionalities ? i assume one (maybe 2 for mirroring use case ?), but want to be sure

This PR is targeted at the FEGA usecase and as such mirroring is not in the scope.
If a dedicated mirroring step is required in addition to backup we might have to bring the backup service back under a different name.

shall we include this in the description and maybe adjust finalize.md ?

Yes the readme needs to be updated.

@jbygdell
Copy link
Collaborator Author

Could you elaborate a bit on how these reflect on the commits of the PR?

Static code tests uses dockerized services instead of mocked backends for DB, MQ and S3.
Functionality tests track the entire path from upload to complete ingestion and dataset release/deprecation.

also why is this needed?

Static tests usually use mocked backeds, but mocking an MQ backend is all but impossible. Even the libray repo uses live backends in their tests.

Removes code and functionality tests for the sda-pipeline folder.

This could maybe benefit from being an epic or at least multiple PRs but it's a bit late now :)

I was also expecting a whole different thing from the title of the ticket. Are we archiving the sda-pipeline repo after this? Would that mean that we're not keeping any history of the merged repo?

How to deal with the history was solved when this repo was created, and all standalone sda-XXX repos that are merged into this will be archived when the merge of a repo is completed. Currently only sda-common and sda-s3proxy is fully merged.

@jbygdell jbygdell added this to the FEGA production milestone Aug 25, 2023
@pontus
Copy link
Contributor

pontus commented Aug 30, 2023

Also unsure if I understand why sda-pipeline still remains with migrated contents here,

@jbygdell
Copy link
Collaborator Author

Also unsure if I understand why sda-pipeline still remains with migrated contents here,

Not every component is migrated yet and until that the old folder will still be around.

@jbygdell jbygdell changed the title Merge pipeline into SDA, part 1 Merge FEGA related components from pipeline into SDA Sep 14, 2023
@jbygdell
Copy link
Collaborator Author

Will split into separate parts.

@jbygdell jbygdell closed this Sep 15, 2023
@jbygdell jbygdell deleted the merge_pipeline branch October 30, 2023 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants